Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

*: test expired certs in client #7824

Merged
merged 2 commits into from
Apr 26, 2017
Merged

*: test expired certs in client #7824

merged 2 commits into from
Apr 26, 2017

Conversation

gyuho
Copy link
Contributor

@gyuho gyuho commented Apr 26, 2017

Trying to have some testing cases for #7784 and #7807.

ClientCertAuth: true,
}

testTLSInfoBad = transport.TLSInfo{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

testTLSInfoExpired

}

testTLSInfoBad = transport.TLSInfo{
KeyFile: "../../integration/fixtures-bad/server-key.pem",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixtures-expired

}

// expect remote errors 'tls: bad certificate'
_, err = integration.NewClientV3TLS(clus.Members[0], &testTLSInfoBad)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to extend the integration package API:

_, err = clientv3.New(clientv3.Config{
    Endpoints: []string{clus.Members[0].GRPCAddr()},
    TLS: &testTLSInfoExpired,
})

clus := integration.NewClusterV3(t, &integration.ClusterConfig{Size: 1, PeerTLS: &testTLSInfo, ClientTLS: &testTLSInfo})
defer clus.Terminate(t)

c1, err := integration.NewClientV3(clus.Members[0])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can this be left out? Seems like other tests should cover the success case already

@fanminshi
Copy link
Member

Maybe change commit title from using the keyword bad to just have the expired keyword.
e.g
*: test bad(expired) certs in client => test expired certs in client

clientv3/integration: test client dial with bad certs => clientv3/integration: test client dial with expired certs

integration/fixtures: add bad certs (expired ones) => integration/fixtures: add expired certs

Signed-off-by: Gyu-Ho Lee <gyuhox@gmail.com>
@gyuho gyuho changed the title *: test bad(expired) certs in client *: test expired certs in client Apr 26, 2017
Signed-off-by: Gyu-Ho Lee <gyuhox@gmail.com>
@gyuho
Copy link
Contributor Author

gyuho commented Apr 26, 2017

@heyitsanthony @fanminshi All addressed. PTAL. Thanks.

@fanminshi
Copy link
Member

lgtm

@gyuho gyuho merged commit 633a0a8 into etcd-io:master Apr 26, 2017
@gyuho gyuho deleted the certs branch April 26, 2017 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants